feat: remove curve oracle from redemption contract #232
Conversation
| round_id: uint80 = 0 | ||
| price: int256 = 0 | ||
| started_at: uint256 = 0 | ||
| updated_at: uint256 = 0 | ||
| answered_in_round: uint80 = 0 | ||
| (round_id, price, started_at, updated_at, answered_in_round) = PRICE_FEED.latestRoundData() | ||
| assert updated_at + 3600 > block.timestamp, "price too old" | ||
| return price | ||
| return convert(price, uint256) |
There was a problem hiding this comment.
For reference the price data is sourced from this aggregate contract
https://etherscan.io/address/0x3ebeaca272ce4f60e800f6c5ee678f50d2882fd4#code
That merges this 2 sources to create a YFI/ETH price
- YFI/USD: 0xA027702dbb89fbd58938e4324ac03B58d812b0E1
- ETH/USD: 0x5f4eC3Df9cbd43714FE2740f5E3616155c5b8419
| @@ -136,13 +136,13 @@ def test_sweep(d_yfi, yfi, redemption, gov): | |||
| def test_oracle(project, yfi, d_yfi, ve_yfi, gov): | |||
| mock = project.MockOracle.deploy(sender=gov) | |||
| redemption = project.Redemption.deploy( | |||
| yfi, d_yfi, ve_yfi, gov, mock, mock, 10 * AMOUNT, sender=gov | |||
| yfi, d_yfi, ve_yfi, gov, mock, 10 * AMOUNT, sender=gov | |||
There was a problem hiding this comment.
since unit tests mostly using the mock oracle and we had the contract live we have some sample data of actual values return
is it to much work to add to ape a fork test using this same account and value and check that return with new contract matches reedem expectations?
since test can be integration only on specific block number, to check how this contract new only CL feed would have behaved against these past trxs
one example
https://etherscan.io/tx/0x51613ef0c083e8c133a2d71fa628db4f4c80c8462e2b5efce05f0f3b0faa9d78
we could use others i guess
There was a problem hiding this comment.
I have added a test here that compares the oracle price in the old and new redemption contract, after making sure the old one uses the chainlink price. This should be an equivalent test without having to run a test on an old block, which is (afaik) not very easy to do
|
CI is timing out but all tests pass locally: |
Previously the YFI price was taken as
max(chainlink_price, curve_price), this PR changes it tochainlink_priceonly